feat(benchmark): add jumpi-intense test case#1693
Conversation
|
CC either @jsign @jochem-brouwer @chfast |
jochem-brouwer
left a comment
jochem-brouwer
left a comment
There was a problem hiding this comment.
Agree with the comments from @chfast, also have a small style suggestion, plus a suggestion for a test where the JUMPI performs the jump (instead of "falling through" and thus not performing the jump).
jsign
added
scope:tests
jochem-brouwer
left a comment
jochem-brouwer
left a comment
There was a problem hiding this comment.
Left some extra information and some comments, hope it is helpful 😄 👍
|
@jochem-brouwer Thank you so much for the explanation, it really helped me understand the inner workings of the test better. I’ve updated accordingly, and this PR is now ready for review. It would be great if you could take another look. Thanks again! |
jochem-brouwer
left a comment
jochem-brouwer
left a comment
There was a problem hiding this comment.
Thank you so much for the explanation, it really helped me understand the inner workings of the test better.
Love that it was so helpful 😄 👍 Always happy to help 😄
I left two small comments for some tiny improvements 👍
|
@jochem-brouwer Thanks for the review, update accordingly! |
spencer-tb
changed the title
|
#1804 got merged! Please rebase the branch and move the tests to the appropriate folder. |
marioevz
left a comment
marioevz
left a comment
There was a problem hiding this comment.
LGTM! Just a couple of comments, a rebase to main due to conflicts, and then we can merge.
marioevz
force-pushed
the
feat/add-zkevm-jumpi
branch
from
dbd8af9 to
23be0d8
Compare
* feat: add zkevm jumpi-intense op * refactor: update jumpi fallthrough case * feat(tests): add jumpi intensive contract test * chore: fix typo * refactor: optimize jumpi-intensive test case * refactor(tests): replace hardcoded value with fork specific variable * Apply suggestions from code review --------- Co-authored-by: Mario Vega <marioevz@gmail.com>
8 participants
🗒️ Description
Add a JUMPI-intensive operation for ZKVm as test case.
Currently
🔗 Related Issues
Update issue #1690
✅ Checklist
mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.